Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use XDG specification environment variables #357

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

LucasFA
Copy link
Contributor

@LucasFA LucasFA commented Feb 5, 2024

I noticed a while back that the default XDG base specification locations are hard coded. This PR changes the project to use the corresponding environment variables.

This is a now obsolete message, but let's keep it for context This was when considering to use the dirs-next crate: ~~As I comment in the commit message~~, it is a BREAKING CHANGE for users with the XDG_CONFIG_HOME variable set to one different from the default. Note that while the spec seems relatively Linux-centric, the dirs-next crate also uses recommended guidelines in both mac OS and Windows and so would ~~be breaking for possibly absolutely~~ affect all users. See the table:
Linux	 $XDG_CONFIG_HOME or $HOME/.config	/home/alice/.config
macOS	 $HOME/Library/Application Support	/Users/Alice/Library/Application Support
Windows	 {FOLDERID_RoamingAppData}	        C:\Users\Alice\AppData\Roaming

Obviously breaking all users' config is breaking for any release, so I thought about renaming the current files to whatever they would become after the new location, but that might just break other users, for all I know. (I'm looking at you, NixOS)

Then this is some contextual information from what other projects do:

Instead of doing that, a more reasonable solution is simply using the old config location in case it exists, or otherwise default to the new location. I think this is what git did ( https://git-scm.com/docs/git-config#Documentation/git-config.txt-XDGCONFIGHOMEgitconfig ), only they go on to read the non-compliant file anyway.

It seem that while the convention is clearly defined in Linux, the situation is not so clear in MacOS, and somewhat in Windows. In Mac, ome people strongly prefer the platform recommended locations, and other strongly prefer the XDG ~/.config location. There's precendent: https://github.com/rust-lang/rfcs/pull/1615#issuecomment-221361170, https://github.com/dirs-dev/directories-rs/issues/47. Neovim seems to clump all Unix together, so they also use ~/.config for MacOS. On the opposite end is the `dirs-next` crate, which doesn't honor the XDG environment variables in MacOS: [docs](https://docs.rs/dirs-next/2.0.0/dirs_next/fn.cache_dir.html)

Current status

This PR just detects if the XDG_CONFIG_HOME and XDG_CACHE_HOME environment variable are set. For compatibililty, if XDG_CONFIG_HOME is set but the old location exists, keep current behaviour. Cache is migrated to XDG_CACHE_HOME always.

Given that the cache folder is not routinely accessed, this should not affect any users negatively.

@LucasFA LucasFA force-pushed the xdg-config branch 2 times, most recently from 307dade to d342513 Compare February 5, 2024 23:00
@LucasFA LucasFA force-pushed the xdg-config branch 3 times, most recently from 60cebf8 to 7854c12 Compare March 2, 2024 18:22
Use original config location if it exists, falls back on XDG.

Users will see no change if they haven't set any XDG_* environment
variable.

Everyone will have the option to establish their settings in their
$XDG_CONFIG_HOME directory and their cache location analogously.

No platform specific locations are used for simplicity.
@leon332157
Copy link
Contributor

Is there any update for this PR? I would like to address a small issue with joining path on Windows

@LucasFA
Copy link
Contributor Author

LucasFA commented Jul 30, 2024

Hi @leon332157, I'm afraid I left this hanging. Feel free to take the PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants